Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Picker): should reset the second argument when the first argument… #13366

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

JerryWu1234
Copy link

@JerryWu1234 JerryWu1234 commented Feb 26, 2025

… is changed somehow

fixes: #13365

we should reset the second argument when the first argument is changed.

Before submitting a pull request, please read the contributing guide.

在提交 pull request 之前,请阅读 贡献指南

@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.77%. Comparing base (ec5b45b) to head (3e2ec4d).
Report is 44 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13366      +/-   ##
==========================================
+ Coverage   89.60%   89.77%   +0.16%     
==========================================
  Files         257      257              
  Lines        7013     7021       +8     
  Branches     1736     1738       +2     
==========================================
+ Hits         6284     6303      +19     
+ Misses        384      380       -4     
+ Partials      345      338       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JerryWu1234
Copy link
Author

@inottn Please help to review. I done and fixed all fail tests .

@inottn
Copy link
Collaborator

inottn commented Feb 27, 2025

I don't know if the previous interactions were intentional; if there is an issue, the onChange logic might need to be modified. If I have time, I will see how to adjust it.

@JerryWu1234
Copy link
Author

JerryWu1234 commented Feb 27, 2025

I don't know if the previous interactions were intentional; if there is an issue, the onChange logic might need to be modified. If I have time, I will see how to adjust it.

I definitely sure that it wasn't intentional. and it is a bug.
I would like do it if you are busy.

@JerryWu1234
Copy link
Author

JerryWu1234 commented Mar 1, 2025

thanks for point out.
now no Area component error, and added edge case.
please help to review it again
@inottn

@chenjiahan
Copy link
Member

I prefer to keep the current behavior. If we reset the option it will break the user experience of date picker or time picker.

@JerryWu1234
Copy link
Author

I prefer to keep the current behavior. If we reset the option it will break the user experience of date picker or time picker.

ok, if we keep the current behavior.
Is there another workaround to fix it?
@chenjiahan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] picker切换的时候,如果有子元素的时候,子元素都是从0开始的
4 participants